Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add vertex trimming to the phase2 hlt tracking iterations #47113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lguzzi
Copy link
Contributor

@lguzzi lguzzi commented Jan 16, 2025

PR description:

This PR adds vertex trimming to the phase2 hlt menu with a proc. modifier. This modifier allows to select and build only initial step seeds and high-pt triplet step doublets compatible with a subset of pixel vertices.
Preliminary studies of this configuration are available here for the legacy iterations.

PR validation:

This PR has been validated comparing the results of the customisation function used so far with the proc. modifier approach. Comparison plots show no difference.
Efficiency and fake rates for the alpaka and LST configurations are outlined here.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

not needed

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 16, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lguzzi for master.

It involves the following packages:

  • Configuration/ProcessModifiers (operations)
  • HLTrigger/Configuration (hlt)

@Martin-Grunewald, @antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @mmusich, @rappoccio can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @SohamBhattacharya, @VourMa, @fabiocos, @makortel, @missirol, @mmusich, @rovere this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here


# This modifier is used to enable vertex trimming in the tracking sequences
# and in all related reconstruction modules
vertexTrimming = cms.Modifier()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be renamed to signal it's about the phase-2 HLT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed it to phase2_hlt_vertexTrimming in c05a4f2

scalingStartNPix = cms.double(0.0),
scalingEndNPix = cms.double(1.0),
),
mightGet = cms.optional.untracked.vstring,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mightGet = cms.optional.untracked.vstring,

unneeded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done d17f0fb

useFakeVertices = cms.bool(False),
maxNVertices = cms.int32(-1),
nSigmaZ = cms.double(4.0),
pixelClustersForScaling = cms.InputTag('siPixelClusters'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an offline product?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I copied it from the defaults. It is not used in this configuration, however I changed it to hltSiPixelClusters in d17f0fb

@mmusich
Copy link
Contributor

mmusich commented Jan 16, 2025

test parameters:

  • relvals_opt = --what upgrade
  • workflows = 29634.753, 29634.754, 29634.755, 29634.756, 29634.757, 29634.758, 29634.759, 29634.760, 29634.761
  • enable = hlt_p2_integration, hlt_p2_timing

@mmusich
Copy link
Contributor

mmusich commented Jan 16, 2025

@lguzzi please add workflows that can exercise the trimming.
You can take inspiration from the other variants:

upgradeWFs['HLTTiming75e33Alpaka'] = deepcopy(upgradeWFs['HLTTiming75e33'])
upgradeWFs['HLTTiming75e33Alpaka'].suffix = '_HLT75e33TimingAlpaka'
upgradeWFs['HLTTiming75e33Alpaka'].offset = 0.751
upgradeWFs['HLTTiming75e33Alpaka'].step2 = {
'-s':'DIGI:pdigi_valid,L1TrackTrigger,L1,L1P2GT,DIGI2RAW,HLT:75e33_timing',
'--procModifiers': 'alpaka'
}
upgradeWFs['HLTTiming75e33TiclV5'] = deepcopy(upgradeWFs['HLTTiming75e33'])
upgradeWFs['HLTTiming75e33TiclV5'].suffix = '_HLT75e33TimingTiclV5'
upgradeWFs['HLTTiming75e33TiclV5'].offset = 0.752
upgradeWFs['HLTTiming75e33TiclV5'].step2 = {
'-s':'DIGI:pdigi_valid,L1TrackTrigger,L1,L1P2GT,DIGI2RAW,HLT:75e33_timing',
'--procModifiers': 'ticl_v5'
}
upgradeWFs['HLTTiming75e33AlpakaSingleIter'] = deepcopy(upgradeWFs['HLTTiming75e33'])
upgradeWFs['HLTTiming75e33AlpakaSingleIter'].suffix = '_HLT75e33TimingAlpakaSingleIter'
upgradeWFs['HLTTiming75e33AlpakaSingleIter'].offset = 0.753
upgradeWFs['HLTTiming75e33AlpakaSingleIter'].step2 = {
'-s':'DIGI:pdigi_valid,L1TrackTrigger,L1,L1P2GT,DIGI2RAW,HLT:75e33_timing',
'--procModifiers': 'alpaka,singleIterPatatrack'
}
upgradeWFs['HLTTiming75e33AlpakaSingleIterLST'] = deepcopy(upgradeWFs['HLTTiming75e33'])
upgradeWFs['HLTTiming75e33AlpakaSingleIterLST'].suffix = '_HLT75e33TimingAlpakaSingleIterLST'
upgradeWFs['HLTTiming75e33AlpakaSingleIterLST'].offset = 0.754
upgradeWFs['HLTTiming75e33AlpakaSingleIterLST'].step2 = {
'-s':'DIGI:pdigi_valid,L1TrackTrigger,L1,L1P2GT,DIGI2RAW,HLT:75e33_timing',
'--procModifiers': 'alpaka,singleIterPatatrack,trackingLST'
}
upgradeWFs['HLTTiming75e33AlpakaLST'] = deepcopy(upgradeWFs['HLTTiming75e33'])
upgradeWFs['HLTTiming75e33AlpakaLST'].suffix = '_HLT75e33TimingAlpakaLST'
upgradeWFs['HLTTiming75e33AlpakaLST'].offset = 0.755
upgradeWFs['HLTTiming75e33AlpakaLST'].step2 = {
'-s':'DIGI:pdigi_valid,L1TrackTrigger,L1,L1P2GT,DIGI2RAW,HLT:75e33_timing',
'--procModifiers': 'alpaka,trackingLST'
}

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43321

  • Found files with invalid states:
    • Configuration/ProcessModifiers/python/vertexTrimming_cff.py:

@cmsbuild
Copy link
Contributor

Pull request #47113 was updated. @Martin-Grunewald, @antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @mmusich, @rappoccio can you please check and sign again.

# This modifier is used to enable vertex trimming in the tracking sequences
# and in all related reconstruction modules
phase2_hlt_vertexTrimming = cms.Modifier()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is uneeded @lguzzi. Anyway please rebase to have just a single commit (also add the requested workflows).

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43322

  • Found files with invalid states:
    • Configuration/ProcessModifiers/python/vertexTrimming_cff.py:

@cmsbuild
Copy link
Contributor

Pull request #47113 was updated. @Martin-Grunewald, @antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @mmusich, @rappoccio can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #47113 was updated. @AdrianoDee, @Martin-Grunewald, @Moanwar, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @mandrenguyen, @miquork, @mmusich, @rappoccio, @srimanob, @subirsarkar can you please check and sign again.

@lguzzi
Copy link
Contributor Author

lguzzi commented Jan 28, 2025

@smuzaffar the last force push should fix the workflows.

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals
Size: This PR adds an extra 36KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ee3df1/44001/summary.html
COMMIT: b5273ef
CMSSW: CMSSW_15_0_X_2025-01-28-1100/el8_amd64_gcc12
Additional Tests: HLT_P2_INTEGRATION,HLT_P2_TIMING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47113/44001/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

ValueError: Undefined workflows: 29634.761, 29634.758, 29634.757, 29634.759

* 0.758 HLT phase-2 timing menu Alpaka, trimmed tracking, single tracking iteration variant
* 0.759 HLT phase-2 timing menu trimmed tracking, LST building variant
* 0.760 HLT phase-2 timing menu Alpaka, trimmed tracking, LST building variant
* 0.761 HLT phase-2 timing menu Alpaka, trimmed tracking, single tracking iteration variant, LST building variant
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of these are outdated now.

@mmusich
Copy link
Contributor

mmusich commented Jan 28, 2025

test parameters:

  • relvals_opt = --what upgrade
  • workflows = 29634.753, 29634.754, 29634.755, 29634.756, 29634.7561, 29634.7562, 29634.7563, 29634.7564, 29634.7565
  • enable = hlt_p2_integration, hlt_p2_timing

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43467

@cmsbuild
Copy link
Contributor

Pull request #47113 was updated. @AdrianoDee, @Martin-Grunewald, @Moanwar, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @mandrenguyen, @miquork, @mmusich, @rappoccio, @srimanob, @subirsarkar can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Jan 29, 2025

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 40KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ee3df1/44018/summary.html
COMMIT: 8525b14
CMSSW: CMSSW_15_0_X_2025-01-28-2300/el8_amd64_gcc12
Additional Tests: HLT_P2_INTEGRATION,HLT_P2_TIMING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47113/44018/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 1 lines to the logs
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4016770
  • DQMHistoTests: Total failures: 60
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4016690
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 218 log files, 189 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

picky, cms specific types could be dropped if they pre-exist in the modified configuration.

@@ -12,3 +12,6 @@
trackingRegions = cms.InputTag("hltPhase2PixelTracksAndHighPtStepTrackingRegions"),
trackingRegionsSeedingLayers = cms.InputTag("")
)

from Configuration.ProcessModifiers.phase2_hlt_vertexTrimming_cff import phase2_hlt_vertexTrimming
phase2_hlt_vertexTrimming.toModify(hltHighPtTripletStepHitDoublets, trackingRegions = cms.InputTag('hltTrackingRegionFromTrimmedVertices'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
phase2_hlt_vertexTrimming.toModify(hltHighPtTripletStepHitDoublets, trackingRegions = cms.InputTag('hltTrackingRegionFromTrimmedVertices'))
phase2_hlt_vertexTrimming.toModify(hltHighPtTripletStepHitDoublets, trackingRegions = "hltTrackingRegionFromTrimmedVertices")

@@ -17,3 +17,6 @@

from Configuration.ProcessModifiers.trackingLST_cff import trackingLST
trackingLST.toModify(hltInitialStepSeeds, includeFourthHit = True)

from Configuration.ProcessModifiers.phase2_hlt_vertexTrimming_cff import phase2_hlt_vertexTrimming
phase2_hlt_vertexTrimming.toModify(hltInitialStepSeeds, InputVertexCollection = cms.InputTag("hltPhase2TrimmedPixelVertices"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
phase2_hlt_vertexTrimming.toModify(hltInitialStepSeeds, InputVertexCollection = cms.InputTag("hltPhase2TrimmedPixelVertices"))
phase2_hlt_vertexTrimming.toModify(hltInitialStepSeeds, InputVertexCollection = "hltPhase2TrimmedPixelVertices")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants